-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modify compound ingest operation to wait for table build completion #970
Conversation
""":str: The status of the job. One of "Running", "Success", or "Failure".""" | ||
tasks = properties.List(Object(TaskNode), "tasks") | ||
""":List[TaskNode]: all of the constituent task required to complete this job""" | ||
output = properties.Optional(properties.Mapping(String, String), 'output') | ||
""":Optional[dict[str, str]]: job output properties and results""" | ||
|
||
@property | ||
def status(self) -> Union[JobStatus, str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since BaseEnumeration
has str
as a superclass, returning a JobStatus
would not lead to a change in behavior, so this is not an API break.
src/citrine/jobs/job.py
Outdated
return self._status | ||
|
||
@status.setter | ||
def status(self, value): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: type hint
@status.setter | ||
def status(self, value): | ||
if resolved := JobStatus.from_str(value, exception=False): | ||
if resolved not in [JobStatus.RUNNING, JobStatus.SUCCESS, JobStatus.FAILURE]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this support any valid JobStatus
value? If so, you can use list(JobStatus)
to ensure it's resilient against typos and future code changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. That's strange, that the Response
can't take the other statuses. Not even Pending
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what's documented. This ought to just be a read only object, so maybe we don't care about the fact that the far end will only ever return a subset.
Citrine Python PR
Description
This PR updates the poll_for_job_completion method in ingestion to continue blocking until a table is built if the build_table option was true.
This PR also
PR Type:
Adherence to team decisions